Conversation
58b1095 to
679cc1f
Compare
679cc1f to
0482e15
Compare
0482e15 to
671e192
Compare
671e192 to
f553c07
Compare
f9bbf83 to
589bca9
Compare
589bca9 to
2b01b20
Compare
| fun String.appendQuery(query: String): String { | ||
| val querySeparator = if (contains("?")) "&" else "?" | ||
| return this + querySeparator + query | ||
| } |
There was a problem hiding this comment.
Why not use Uri.Builder or buildUrl from ktor to manage all the case of ApiRoutes ?
| with(publicShareViewModel) { | ||
| val emptyFilesResult = PublicShareFilesResult(files = emptyList(), shouldUpdate = false, isNewSort = false) | ||
| childrenLiveData.value = emptyFilesResult | ||
| cancelDownload() | ||
|
|
||
| if (folderId == ROOT_SHARED_FILE_ID || rootSharedFile.value == null) { | ||
| if (folderId == ROOT_SHARED_FILE_ID || !isFolder()) { | ||
| downloadPublicShareRootFile() | ||
| } else { | ||
| getFiles(folderId, fileListViewModel.sortType, isNewSort) |
There was a problem hiding this comment.
Could be all moved in a method in PublicShareViewModel, what do you think ?
2b01b20 to
eaec8d5
Compare
…ymore The problem was because fileId came directly from the activity navargs, but when coming from the password fragment, it was not updated. So the root file download had a bad id
…ate the risk of taking it for the same thing as folderId
…ave the data for the previews/thumbnails
eaec8d5 to
a9b8f4d
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR enables opening password-protected public shares directly in the Android app by adding a password validation flow that retrieves and propagates a share-link auth token across public-share API calls and file operations.
Changes:
- Replace the “not supported” password screen with an in-app password submission/validation flow and navigation into the public share file list.
- Introduce a
PublicShareTokenresponse model and propagatesharelink_tokenthrough public-share API routes (listing, counts, archives, previews/downloads). - Attach public-share auth context (UUID + token) to
Fileobjects so downstream features (thumbnails, previews, downloads, counts) can work for protected shares.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Removes obsolete “password not supported” string. |
| app/src/main/res/values-it/strings.xml | Same string removal + copyright year bump. |
| app/src/main/res/values-fr/strings.xml | Same string removal + copyright year bump. |
| app/src/main/res/values-es/strings.xml | Same string removal + copyright year bump. |
| app/src/main/res/values-de/strings.xml | Same string removal + copyright year bump. |
| app/src/main/res/layout/fragment_public_share_password.xml | Updates UI copy/visibility/button label for password entry & validation. |
| app/src/main/java/com/infomaniak/drive/utils/PreviewUtils.kt | Uses safer query appending for OnlyOffice PDF downloads. |
| app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicShareViewModel.kt | Adds token-aware init/root file fetch/listing/import/archive behavior; tracks rootFileId. |
| app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicSharePasswordFragment.kt | Implements in-app password validation and navigates to list on success. |
| app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicShareMultiSelectActionsBottomSheetDialog.kt | Adds token to public-share archive download URL generation. |
| app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicShareListFragment.kt | Updates navigation and root file handling for password-protected entry flow. |
| app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicShareActivity.kt | Initializes rootFileId from navigation args. |
| app/src/main/java/com/infomaniak/drive/ui/fileList/FileListViewModel.kt | Adds token to public-share file count requests. |
| app/src/main/java/com/infomaniak/drive/ui/LaunchActivity.kt | Copyright year bump only. |
| app/src/main/java/com/infomaniak/drive/data/models/File.kt | Adds publicShareAuthToken field (non-persisted/non-parceled). |
| app/src/main/java/com/infomaniak/drive/data/api/publicshare/PublicShareToken.kt | New parcelable model for password auth response token. |
| app/src/main/java/com/infomaniak/drive/data/api/publicshare/PublicShareApiRepository.kt | Adds token support across public-share endpoints and centralizes URL token injection. |
| app/src/main/java/com/infomaniak/drive/data/api/ApiRoutes.kt | Adds token-aware public-share URL helpers + appendQuery utility. |
| app/src/main/java/com/infomaniak/drive/MatomoDrive.kt | Adds ValidatePassword tracking event and removes unused OpenInBrowser. |
Comments suppressed due to low confidence (1)
app/src/main/java/com/infomaniak/drive/ui/publicShare/PublicSharePasswordFragment.kt:95
observeSubmitPasswordResulttreats any empty token as a wrong password and clears the field. With the current ViewModel behavior, an empty token also happens on network/server errors, so users will get a “wrong password” message in those cases. Consider observing a richer result (token + error) and showing an appropriate error message when the auth call fails for reasons other than an invalid password.
publicShareViewModel.submitPasswordResult.observe(viewLifecycleOwner) { authToken ->
if (authToken.isNotEmpty()) {
publicShareViewModel.hasBeenAuthenticated = true
publicShareViewModel.initPublicShare(authToken)
} else {
passwordValidateButton.hideProgressCatching(R.string.buttonValid)
publicSharePasswordEditText.text = null
publicSharePasswordLayout.error = getString(R.string.errorWrongPassword)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fun getPublicShareChildrenFiles( | ||
| driveId: Int, | ||
| linkUuid: String, | ||
| fileId: Int, | ||
| sortType: SortType, | ||
| authToken: String?, | ||
| ): String { | ||
| val orderQuery = "order_by=${sortType.orderBy}&order=${sortType.order}" | ||
| return "$SHARE_URL_V3/$driveId/share/$linkUuid/files/$fileId/files?$sharedFileWithQuery&$orderQuery" | ||
| val authParam = authToken?.let { "&sharelink_token=$it" } ?: "" | ||
| return "$SHARE_URL_V3/$driveId/share/$linkUuid/files/$fileId/files?$sharedFileWithQuery&$orderQuery$authParam" |
There was a problem hiding this comment.
getPublicShareChildrenFiles appends sharelink_token directly into the URL without URL-encoding. If the token contains reserved characters, the resulting URL can be invalid or interpreted incorrectly. Prefer building the query via appendQuery(...) (with proper encoding) or using an HttpUrl builder; also avoid appending when the token is blank.
| private fun getPublicShareFileThumbnail(driveId: Int, linkUuid: String, fileId: Int, authToken: String? = null): String { | ||
| val authParam = authToken?.let { "?sharelink_token=$it" } ?: "" | ||
| return "${publicShareFile(driveId, linkUuid, fileId)}/thumbnail$authParam" | ||
| } | ||
|
|
||
| private fun getPublicShareFilePreview(driveId: Int, linkUuid: String, fileId: Int): String { | ||
| return "${publicShareFile(driveId, linkUuid, fileId)}/preview" | ||
| private fun getPublicShareFilePreview(driveId: Int, linkUuid: String, fileId: Int, authToken: String? = null): String { | ||
| val authParam = authToken?.let { "?sharelink_token=$it" } ?: "" | ||
| return "${publicShareFile(driveId, linkUuid, fileId)}/preview$authParam" | ||
| } | ||
|
|
||
| private fun downloadPublicShareFile(driveId: Int, linkUuid: String, fileId: Int): String { | ||
| return "${publicShareFile(driveId, linkUuid, fileId)}/download" | ||
| private fun downloadPublicShareFile(driveId: Int, linkUuid: String, fileId: Int, authToken: String? = null): String { | ||
| val authParam = authToken?.let { "?sharelink_token=$it" } ?: "" | ||
| return "${publicShareFile(driveId, linkUuid, fileId)}/download$authParam" | ||
| } | ||
|
|
||
| private fun showPublicShareOfficeFile(driveId: Int, linkUuid: String, fileId: Int): String { | ||
| // For now, this call fails because the back hasn't dev the conversion of office files to pdf for mobile | ||
| return "$SHARE_URL_V1/share/$driveId/$linkUuid/preview/text/$fileId" | ||
| private fun showPublicShareOfficeFile(driveId: Int, linkUuid: String, fileId: Int, authToken: String? = null): String { | ||
| val authParam = authToken?.let { "?sharelink_token=$it" } ?: "" | ||
| return "$SHARE_URL_V1/share/$driveId/$linkUuid/preview/text/$fileId$authParam" |
There was a problem hiding this comment.
Several helpers (getPublicShareFileThumbnail, getPublicShareFilePreview, downloadPublicShareFile, showPublicShareOfficeFile, downloadPublicShareArchive) build ?sharelink_token=$it via raw string concatenation. This should URL-encode the token and skip appending when it’s blank to avoid malformed URLs and accidental “empty token” requests.
| driveId = driveId, | ||
| publicShareUuid = publicShareUuid, | ||
| archiveUuid = it.uuid, | ||
| authToken = submitPasswordResult.value, |
There was a problem hiding this comment.
downloadPublicShareArchive is passed submitPasswordResult.value directly. Since this value can be an empty string, this may generate URLs containing ?sharelink_token= (empty token), which can lead to avoidable backend errors. Consider passing submitPasswordResult.value?.takeIf { it.isNotBlank() } (or storing the token as nullable state) so the token is only included when present.
| authToken = submitPasswordResult.value, | |
| authToken = submitPasswordResult.value?.takeIf { token -> token.isNotBlank() }, |
| fun submitPublicSharePassword(password: String) = viewModelScope.launch { | ||
| val passwordResult = PublicShareApiRepository.submitPublicSharePassword( | ||
| val token = PublicShareApiRepository.submitPublicSharePassword( | ||
| driveId = driveId, | ||
| linkUuid = publicShareUuid, | ||
| password = password, | ||
| ).data | ||
| ).data?.token ?: "" | ||
|
|
||
| submitPasswordResult.postValue(passwordResult) | ||
| submitPasswordResult.postValue(token) |
There was a problem hiding this comment.
submitPublicSharePassword posts an empty token for any non-success response (network/server error included). The UI then treats an empty token as a wrong password, which is misleading and can block access when the password is correct but the request failed. Consider propagating the API error separately (e.g., a sealed result or Pair<ApiError?, String?>) and only emitting a token when the response is successful; also avoid using an empty string to represent “no token” (prefer null or takeIf { it.isNotBlank() }).
| val apiResponse = PublicShareApiRepository.getPublicShareInfo(driveId, publicShareUuid) | ||
| fun initPublicShare(authToken: String? = null) = viewModelScope.launch { | ||
| val apiResponse = PublicShareApiRepository.getPublicShareInfo(driveId, publicShareUuid, authToken) | ||
| val result = if (apiResponse.isSuccess()) null to apiResponse.data else apiResponse.error to null |
There was a problem hiding this comment.
initPublicShare treats any isSuccess() response as success even if data is null, which can lead to navigating with PUBLIC_SHARE_DEFAULT_ID (e.g., shareLink?.fileId ?: PUBLIC_SHARE_DEFAULT_ID) and subsequent invalid API calls. Consider treating “success with null body” as an error (post a non-null error / generic error) to prevent proceeding without a valid ShareLink.
| val result = if (apiResponse.isSuccess()) null to apiResponse.data else apiResponse.error to null | |
| val result = if (apiResponse.isSuccess() && apiResponse.data != null) { | |
| null to apiResponse.data | |
| } else { | |
| (apiResponse.error ?: ApiError()) to null | |
| } |



Add the possibility to open a password protected public share directly in the application instead of going to the browser.